Overall change looks ok to me. 👌🏼

It looks like ☑️ it passed the spec https://github.com/zooniverse/panoptes/blob/master/spec/lib/workflow_copier_spec.rb#L35-L38 which checks to make sure non-excluded attributes are being included. ☑️ and it passes your new spec that attached_images be included ☑️ passes the spec that any excluded attributes are set to defaults for the copied workflow.

OOC, do you know why deep_cloneable gem's #deep_clone is not including attached images? It feels like it should without us having to explicitly include it. But current behavior in panoptes has proved otherwise. 🤔 (I havent looked too much into this, but was wondering if you have.) @Tooyosi

Yeah, my understanding is it has to be included for it to be copied along, here: https://github.com/moiristo/deep_cloneable?tab=readme-ov-file#association-inclusion

Super sorry I should have caught this sooner when looking through spec.

Are we certain that workflow.attached_images in spec has media (i.e. are we certain workflow.attached_images is not an empty array?) ?

I worry that the way we define media with let and how let gets lazily evaluated, that the media is actually not getting associated with the workflow until invoked (i.e. until we specifically call out media eg. puts media). Maybe let! or instead setting/creating media within the test it copies the attached_images would get media associated to workflow?

Thanks for picking this, somehow it seems like it was good originally but i can see the issue. I have moved the creation into the specific test case and checking the count as both items would not be the exact same records (i.e different id, created date e.t.c)